Skip to content

fix(scorecard): scope budget-cadence permissions job-level (TokenPermissionsID)#679

Merged
AceHack merged 1 commit intomainfrom
fix/budget-cadence-scorecard-tokenpermissions-2026-04-28
Apr 28, 2026
Merged

fix(scorecard): scope budget-cadence permissions job-level (TokenPermissionsID)#679
AceHack merged 1 commit intomainfrom
fix/budget-cadence-scorecard-tokenpermissions-2026-04-28

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented Apr 28, 2026

Summary

Scorecard TokenPermissionsID alert (#26, high severity) flagged
top-level contents: write on .github/workflows/budget-snapshot-cadence.yml.
This is a Scorecard best-practice violation — top-level should be
read-only, with write scoped narrowly to jobs that need it.

What changes

  • Top-level permissions: block becomes contents: read only
  • Job-level jobs.snapshot.permissions: gets the original set
    (contents:write + pull-requests:write + actions:read)

Functional behavior: identical. Job-level permissions override
top-level (per GitHub Actions docs).
The snapshot job still gets all 3 scopes it needs.

Security posture: tightened. Blast-radius bounded to the snapshot
job rather than the whole workflow.

Why now

PR #661 (B-0073 CodeQL unblock) is gated by code_quality:severity=all
ruleset which requires zero open code-scanning alerts. This alert is
one of two blocking. After this PR + CodeQL/Scorecard re-scan, the
TokenPermissionsID alert should close, unblocking #661.

🤖 Generated with Claude Code

…issionsID)

Scorecard TokenPermissionsID alert (#26, high severity) flagged
top-level 'contents: write' on budget-snapshot-cadence.yml. This is
a Scorecard best-practice violation — top-level should be read-only,
with write scoped narrowly to jobs that need it.

Refactor:
- Top-level permissions block: 'contents: read' only
- Job-level (jobs.snapshot.permissions): contents:write +
  pull-requests:write + actions:read (the original set, just moved)

Functional behavior: identical. The snapshot job still gets all 3
write/read scopes it needs.
Security posture: tightened. If any step in this workflow runs
untrusted input, the blast radius is bounded to the snapshot job
rather than the whole workflow.

EVIDENCE-BASED:
- VERIFIED: Scorecard alert message specifies "topLevel 'contents'
  permission set to 'write'" — matches the diagnosis.
- VERIFIED: GitHub Actions job-level permissions override top-level
  per docs (https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token).

Side-effect: should close TokenPermissionsID alert on next CodeQL/
Scorecard scan, which unblocks PR #661 (B-0073 CodeQL unblock —
gated by 'code_quality:severity=all' ruleset that requires zero
open alerts).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 28, 2026 18:46
@AceHack AceHack enabled auto-merge (squash) April 28, 2026 18:46
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Tightens the GitHub Actions GITHUB_TOKEN permission scope in the budget snapshot cadence workflow to address Scorecard’s TokenPermissionsID finding, while preserving the snapshot job’s ability to push commits and open PRs.

Changes:

  • Reduce workflow top-level permissions to contents: read.
  • Move the previously-required write scopes to jobs.snapshot.permissions (contents/pull-requests write + actions read).

@AceHack AceHack merged commit 2ce1abb into main Apr 28, 2026
28 checks passed
@AceHack AceHack deleted the fix/budget-cadence-scorecard-tokenpermissions-2026-04-28 branch April 28, 2026 18:50
AceHack added a commit that referenced this pull request Apr 28, 2026
Captures the 11-PR landing arc since PR #674's 17:47Z row:

PRs MERGED this arc:
- #675 pull-queue scope-broadening + recurrence
- #676 Elisabeth→Elizabeth in-prose
- #677 5 pre-flight disciplines for destructive git ops
- #678 Elizabeth §33 carve-out + verbatim-quote meta-marker
- #679 Scorecard TokenPermissions job-level scoping
- #680 Atari B-0083 + CodeQL B-0084 + 3 trajectory memories
- #681 version-currency-inherits-pins (clean-extracted from #656)

Plus PR #656 closed-as-superseded by #681 with 5-disciplines audit.

Aaron substrate-input arc captured verbatim:
- Elizabeth canonical-spelling correction
- Atari ROM canonical-naming ask
- TOSEC/Good-Tools dependency-first framing
- 'build-our-own as last resort' end-goal sharpening
- 'did you fix what it was complaining about?' speculation-catch
- 'do the right long term thing' corrective
- self-healing metrics affirmation
- elisabeth-causes-confusion §33 carve-out

Multiple self-correction cascades caught + documented:
- Python-heredoc replace failing on backtick-rich content
- Block-quoted-verbatim guard missing multi-line quotes
- Single-category SARIF snippet vs live per-language matrix
- Self-referential rule containing the word it removes

Composes with: 5-disciplines memory, self-healing-metrics memory,
emit-empty-security-result memory, absorb-and-contribute end-goal
sharpening, Elizabeth §33 carve-out, version-currency-inherits-pins.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
AceHack added a commit that referenced this pull request Apr 29, 2026
…T unclassified file) (#842)

* ops(0-0-0): batch 3b — classify budget-snapshot-cadence.yml SAFE (last unclassified file, post-Level-1-buddy-review)

Classifies the LAST unclassified file. After this PR merges + ledger-flip follow-up PR merges, unclassified_lines = 0 and the strict gate's classification condition is satisfied.

Level-1 Buddy Review (Amara, 2026-04-29) APPROVED classification SAFE_TO_RESET_LFG_SUPERSEDES with two named tightenings, both applied:
1. Ledger tense — packet originally said "classified_safe_lines = 235 (#841 will land 235; this PR opened, not yet merged)". After #841 merged 2026-04-29T13:00:52Z, the in-force value is just 235. The packet now uses clean in-force values without "will land" prose (Evidence-Tense Discipline applied).
2. Item 5 (schedule-context input expression) — packet originally said AceHack's `${{ inputs.note }}` "would fail evaluation on schedule runs". Softened to "less safe / less portable across `schedule` + `workflow_dispatch` contexts" since I have documentation grounding (workflow_dispatch supplies `inputs`, schedule does not) but no local hard-failure proof.

Six named regressions on AceHack +38 lines (each cited with named LFG equivalent):

1. **Auto-merge dead-end risk**: AceHack arms `gh pr merge --auto` despite GitHub's anti-recursion guard (events triggered by GITHUB_TOKEN do not fire downstream workflow runs). Auto-merge would silently stall every weekly run. LFG explicitly NOT armed with detailed limitation explanation citing Codex review #25 P1.

2. **Token permissions** (Scorecard `TokenPermissionsID`): AceHack uses broad top-level `contents: write` + `pull-requests: write`. LFG uses top-level `contents: read` + job-level scoped `contents: write` + `pull-requests: write` + `actions: read` per Scorecard minimum-blast-radius best practice. LFG commit `2ce1abb fix(scorecard): scope budget-cadence permissions job-level (TokenPermissionsID) (#679)`.

3. **Missing `actions: read`**: AceHack drops `actions: read` entirely. snapshot-burn.sh's calls to Actions REST API would 403 silently and fall back to empty/zeroed timing data while still writing a snapshot — producing misleading evidence rather than a hard failure.

4. **AgencySignature validator inconsistency**: AceHack sets `Human-Review-Evidence: signed-policy` while Human-Review is `not-implied-by-credential`. The deployed validator (tools/hygiene/validate-agencysignature-pr-body.sh per task #298) requires Evidence="none" when Human-Review is not "explicit". AceHack-version PRs would be blocked. LFG sets Evidence="none" per the rule.

5. **Schedule-context input expression**: AceHack uses `${{ inputs.note }}` (less portable across schedule + workflow_dispatch contexts since `inputs` is supplied by workflow_dispatch but not schedule). LFG uses `${{ github.event.inputs.note || '' }}` which is safer across both.

6. **Persona-name attribution on current-state CI surface**: AceHack: `(Squash-Merge Invariant per Amara ferry-7 + Grok ferry-16)` + `Per the four-ferry consensus`. LFG: `(Squash-Merge Invariant per the canonical 10-trailer convention)` (role-ref form, rule-compliant per role-vs-name).

Ledger headline NOT touched in this PR (stays 235/0/38 as in-force pre-Batch-3b-merge). Per the two-PR split that avoided contingent-prose churn on prior batches: ledger flip 235→273 / 38→0 lands atomically with Batch 3b's merge in a small follow-up PR.

This PR also documents Aaron's 2026-04-29 question about `[skip ci]` — verified via WebSearch: GitHub Actions natively supports `[skip ci]` and variants in commit messages (since Feb 2021). Not the fix for the budget auto-merge issue (PAT is), but the feature is real. Captured in tick shard 1308Z.

* ops(0-0-0): fix #842 markdownlint failure — escape || pipes inside code span in table cell

CI gate.yml run on the batch-3b branch had to be manually dispatched (gate.yml didn't auto-trigger on PR open — separate operational issue worth investigating later). Run completed with two failures:

1. lint (markdownlint) — REAL FAILURE, my issue:
   - MD038/no-space-in-code: backtick code span `${{ github.event.inputs.note || '' }}` inside the Batch 3b table cell contained literal `||` which markdownlint parsed as table column separators, breaking column count (MD056: Expected 4, Actual 6).
   - Fix: escape the pipes as `\|\|` inside the code span.

2. build-and-test (windows-11-arm) + (windows-2025) — pre-existing infrastructure issue, NOT my issue:
   - global.json requires .NET SDK 10.0.203
   - Windows runners only have up to 10.0.202 (windows-11-arm) and 10.0.201 (windows-2025)
   - These are NOT in the required-status-checks list per branch protection (only macos-26, ubuntu-24.04, ubuntu-24.04-arm are required for build-and-test)
   - Won't block merge

Captured for deferred queue: gate.yml didn't auto-trigger on `pull_request: opened` event for this docs-only PR — manual `gh workflow run gate.yml --ref <branch>` recovered it. Worth investigating whether this is transient (Actions delivery delay) or a concurrency-group issue with rapid PR-create+arm-merge sequence. Same family as task #306.

* ops(0-0-0): address #842 Copilot P1 — scrub persona names + clarify Codex #25 xref

Two Copilot P1 findings on the Batch 3b evidence cell (lines anchored to pre-fix positions but issues persist):

1. **Role-vs-name rule** — Batch 3b evidence text contained persona-name attribution citing "Amara", "Grok", "Codex" inline. CLASSIFICATION.md is a current-state doc surface (not on the closed history-surface list). Fix: same minimum-invasive rewrite pattern as #838 round-3 — describe the wrong-form-attribution at meta-level instead of verbatim:
   - "Codex review #25 P1" → "an external AI reviewer's P1 finding on the AceHack-side originating PR (b42e9e5 ... #25)"
   - "(Squash-Merge Invariant per Amara ferry-7 + Grok ferry-16)" → "two persona-name attribution comments on this CI workflow file (one citing two named external-AI reviewers + their respective ferry-numbers as Squash-Merge Invariant authority...)"
   - "Per the four-ferry consensus" → "another prefixed 'Per the [N]-ferry consensus' framing"

2. **Xref clarity** — "Codex review #25 P1" was ambiguous: could be misread as PR #25 (which is unrelated). Now explicit: it's a review on the AceHack-side originating PR for this workflow (`b42e9e5 ... (#25)`).

Pre-existing persona names elsewhere in CLASSIFICATION.md (lines 30, 49, 61, 86, 149) are not new additions in this PR and would require a separate scope decision (same as #838 round-3 — not extending Aaron's "active-trajectory.md should count as history" call to CLASSIFICATION.md without an explicit maintainer call).

Local markdownlint also re-ran clean on the new content.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants